Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
The title in this snapshot now follows the page's md theme instead of the header's mode="ios", since migrated components take their look from the theme and mode only affects behavior; the header itself is still visible, which is what the test checks.
brandyscarney
left a comment
There was a problem hiding this comment.
Looks good! I requested some changes. 🙂
There was a problem hiding this comment.
Why do we need both a color folder and a hue folder? Shouldn't we just have the hue folder since the color has to be either bold or subtle?
| // -------------------------------------------------- | ||
| // We're using the slots to force the main toolbar content to be | ||
| // centered in the toolbar. This is a bit of a hack but it works. | ||
| // centered in the toolbar. This is a bit of a hack butx it works. |
There was a problem hiding this comment.
| // centered in the toolbar. This is a bit of a hack butx it works. | |
| // centered in the toolbar. This is a bit of a hack but it works. |
| /** | ||
| * This behavior does not vary across directions. | ||
| * | ||
| * `ios` is the same as `md`. |
There was a problem hiding this comment.
Why are we skipping ios? All themes will ship with the same colors so eventually they will all be the same.
There was a problem hiding this comment.
Should this be named title.interfaces.ts to match the other recipes?
| height: '100%', | ||
| position: 'relative', | ||
| width: '100%', |
There was a problem hiding this comment.
Why are width and height defined here when they are always 100% for every size?
| */ | ||
| export const isLargeTitle = (ionTitleEl: HTMLIonTitleElement) => { | ||
| const attributeSize = ionTitleEl.getAttribute('size') as IonTitleSize | null; | ||
| const configSize = config.getObjectValue('IonTitle.size') as IonTitleSize | undefined; |
There was a problem hiding this comment.
Non-blocking, possible follow-up for FW-6843
size now does two jobs: it sets how big the title looks, and size="large" also marks the title that collapses (here, and in the ion-title.title-size-large query in ios.transition.ts). Now that size is in the theme config, an app that sets IonTitle: { size: 'large' } would make every title large, including main-header titles. Those would then get the large-title layout and count as collapsing titles.
Could the header migration work out the collapsing title from where it sits rather than from its size? For example, treat the title inside ion-header[collapse="condense"] as the collapsing title. Then size would only control appearance, and an app-wide size default would be harmless.
This wouldn't break anything: size="large" would keep working, it just wouldn't be required, and the "condense requires <ion-title size="large">" warning could go away. Fine to leave as is for this PR.
| width?: string; | ||
| }; | ||
|
|
||
| minWidth?: string; |
There was a problem hiding this comment.
minWidth is only set so the iOS large title wraps onto its own row, and that only works together with the iOS toolbar's flex-wrap. Could it move into toolbar.ios.scss next to that rule as :host(.toolbar-title-size-large) ::slotted(ion-title) { min-width: 100%; } and the token be dropped? That keeps the wrap behavior in one place, and ::slotted reaches the title where the removed .toolbar-content ion-title rule didn't.
| | `toolbar-title-small` | `toolbar-title-size-small` | | ||
| | `toolbar-title-large` | `toolbar-title-size-large` | | ||
|
|
||
| <h5 id="version-10x-title-rtl-class">`title-rtl` class</h5> |
There was a problem hiding this comment.
| <h5 id="version-10x-title-rtl-class">`title-rtl` class</h5> | |
| <h5 id="version-10x-title-rtl-class"><code>title-rtl</code> class</h5> |
| |---|---|---| | ||
| | `--color` | `IonTitle.color` | `--ion-title-color` | | ||
|
|
||
| <h5 id="version-10x-title-hue">New `hue` property and color tokens</h5> |
There was a problem hiding this comment.
| <h5 id="version-10x-title-hue">New `hue` property and color tokens</h5> | |
| <h5 id="version-10x-title-hue">New <code>hue</code> property and color tokens</h5> |
| `ion-toolbar` builds its own classes from these, so they change as well: | ||
|
|
||
| | Old (9.x) | New | | ||
| |---|---| | ||
| | `toolbar-title-default` | `toolbar-title-size-medium` | | ||
| | `toolbar-title-small` | `toolbar-title-size-small` | | ||
| | `toolbar-title-large` | `toolbar-title-size-large` | |
There was a problem hiding this comment.
Should this go in toolbar's section instead?
Issue number: resolves internal
What is the current behavior?
ion-titledoes not fragment styles based on themes.What is the new behavior?
ion-titleto Modular Ionic: one token-driven stylesheet,IonTitleRecipeandIonTitleConfig, and per-theme token defaults for ios, md and ionic.hueprop (boldby default, orsubtle). The ionic theme now appliescolor, matching ios and md.sizeacceptsmedium, which is the default from the newIonTitle.sizeconfig. The header's condense effect also reads that config, so a title made large through it condenses. Size classes are nowtitle-size-small,title-size-mediumandtitle-size-large, and toolbar's matching classes aretoolbar-title-size-*.--colorwith--ion-title-color, which also reaches the copy of a large title shown during the iOS page transition.getIonThemeand thethemevirtual prop) and thetitle-rtlclass.title-largeorder value. The rule's selector can't reach the slotted title, so it never applied, and the title already computesorder: 0. Rendering doesn't change.dynamicFontMaxtoken helper, and rounds scaled sizes indynamicFontMinanddynamicFontMax.title-colorscreenshots now show colored titles. In header's "root iOS header in an MD app" screenshot, the title follows the page's theme: an ancestormodeattribute now only affects behavior, as it already does for every migrated component.Does this introduce a breaking change?
ion-titleCSS variables and classes are replaced by token-driven custom properties and prop-named classes. Use the global token for theme-wide styling, or the CSS variable for component-specific overrides.--colorIonTitle.color--ion-title-colortitle-defaulttitle-size-mediumtitle-smalltitle-size-smalltitle-largetitle-size-largetoolbar-title-defaulttoolbar-title-size-mediumtoolbar-title-smalltoolbar-title-size-smalltoolbar-title-largetoolbar-title-size-largeRTL class:
title-rtlis removed. Useion-title:dir(rtl)instead.Theme classes: remove any instances that target
ion-title.mdorion-title.ios.Other information
Left for the parents
Some
ion-titlestyles still live in, or depend on, its parents. They move with that parent's work, so they aren't part of this PR.titlePlacement: thetext-alignrules intoolbar.ionic.scss, and the iOS title's own centering intitle.scss(marked with TODOs). A follow-up PR stacked on this one makestitlePlacementwork in every theme and removes both.header.common.scssthat hide and fade the title as the large title collapses. They depend on the header's scroll state and include its condense motion, so they move in the header migration (FW-6843).How to test
Open each before and after link side by side.